oxenstored: handle unknown operations by returning an error to the client

Previous an unknown operation would be decoded as a Not_found exception which
would bubble all the way up to the try ... with surrounding the call to
main_loop where it would be logged and ignored.

This would leave the guest hanging waiting for a response to the invalid
request.

Instead introduce a specific "Invalid" operation. Higher level functionality,
such as Process.process_packet, already handles operations which are not
understood with an error reply due to the final wildcard entry in
Process.function_of_type but explicitly handle Invalid this way to make it
clear what is going on.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml
index 1ce72da3482e618cc4715a3a753616248731e5c1..0ee866676f3ee6cef53b391007303b24e094e2d0 100644 (file)
--- a/tools/ocaml/libs/xb/op.ml
+++ b/tools/ocaml/libs/xb/op.ml
@@ -19,8 +19,7 @@ type operation = Debug | Directory | Read | Getperms |
                  Transaction_end | Introduce | Release |
                  Getdomainpath | Write | Mkdir | Rm |
                  Setperms | Watchevent | Error | Isintroduced |
-                 Resume | Set_target
-               | Restrict 
+                 Resume | Set_target | Restrict | Invalid
 
 let operation_c_mapping =
        [| Debug; Directory; Read; Getperms;
@@ -41,7 +40,7 @@ let array_search el a =
 let of_cval i =
        if i >= 0 && i < size
        then operation_c_mapping.(i)
-       else raise Not_found
+       else Invalid
 
 let to_cval op =
        array_search op operation_c_mapping
@@ -69,3 +68,4 @@ let to_string ty =
        | Resume                -> "RESUME"
        | Set_target            -> "SET_TARGET"
        | Restrict              -> "RESTRICT"
+       | Invalid               -> "INVALID"

diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli
index 1dde52db1e317a0c3d9bd3951669791751501792..58234aefed37a1edb7a341b0b5f0e80896cb395d 100644 (file)
--- a/tools/ocaml/libs/xb/xb.mli
+++ b/tools/ocaml/libs/xb/xb.mli
@@ -23,6 +23,7 @@ module Op :
       | Resume
       | Set_target
       | Restrict
+      | Invalid (* Not a valid wire operation *)
     val operation_c_mapping : operation array
     val size : int
     val array_search : 'a -> 'a array -> int

diff --git a/tools/ocaml/xenstored/logging.ml b/tools/ocaml/xenstored/logging.ml
index 84d7c82cbae359fe5b29ca09eca6f518f8f7e079..7152b4ec6c4177a251fef235dbadcb5ea95d025a 100644 (file)
--- a/tools/ocaml/xenstored/logging.ml
+++ b/tools/ocaml/xenstored/logging.ml
@@ -182,6 +182,7 @@ let string_of_access_type = function
 
        | Xenbus.Xb.Op.Error             -> "error    "
        | Xenbus.Xb.Op.Watchevent        -> "w event  "
+       | Xenbus.Xb.Op.Invalid           -> "invalid  "
        (*
        | x                       -> Xenbus.Xb.Op.to_string x
        *)

diff --git a/tools/ocaml/xenstored/process.ml b/tools/ocaml/xenstored/process.ml
index c2aeaa94a43926bd3debb52fbe7b486de26e30e8..a4ff7412642d22f707082fcae453df524dc43f5c 100644 (file)
--- a/tools/ocaml/xenstored/process.ml
+++ b/tools/ocaml/xenstored/process.ml
@@ -324,7 +324,8 @@ let function_of_type ty =
        | Xenbus.Xb.Op.Resume            -> reply_ack do_resume
        | Xenbus.Xb.Op.Set_target        -> reply_ack do_set_target
        | Xenbus.Xb.Op.Restrict          -> reply_ack do_restrict
-       | _                       -> reply_ack do_error
+       | Xenbus.Xb.Op.Invalid           -> reply_ack do_error
+       | _                              -> reply_ack do_error
 
 let input_handle_error ~cons ~doms ~fct ~ty ~con ~t ~rid ~data =
        let reply_error e =

diff --git a/tools/ocaml/xenstored/xenstored.ml b/tools/ocaml/xenstored/xenstored.ml
index a08aa65d33f088839fc69d5ee6cd7093bad36bd3..564dbeaddbf141817d6a6cee9779790c030b5245 100644 (file)
--- a/tools/ocaml/xenstored/xenstored.ml
+++ b/tools/ocaml/xenstored/xenstored.ml
@@ -43,9 +43,7 @@ let process_connection_fds store cons domains rset wset =
                        debug "closing socket connection"
                in
        let process_fdset_with fds fct =
-               List.iter (fun fd ->
-                          try try_fct fct (Connections.find cons fd)
-                          with Not_found -> ()) fds
+               List.iter (fun fd -> try_fct fct (Connections.find cons fd)) fds
        in
        process_fdset_with rset Process.do_input;
        process_fdset_with wset Process.do_output